Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

feat(misconf): loading embedded checks as a fallback #6502

Merged
merged 5 commits into from
Apr 19, 2024
Merged

feat(misconf): loading embedded checks as a fallback #6502

merged 5 commits into from
Apr 19, 2024

Conversation

nikpivkin
Copy link
Contributor

@nikpivkin nikpivkin commented Apr 16, 2024 •
edited
Loading

Description

This PR adds functionality to fallback to embedded checks if an error occurs compiling an built-in check from a bundle.

main.tf

resource "aws_s3_bucket" "test" {
  bucket = "test"
}

resource "aws_s3_bucket" "log_bucket" {
  bucket = "log_bucket"
}

resource "aws_s3_bucket_logging" "test" {
  bucket = aws_s3_bucket.test.id

  target_bucket = aws_s3_bucket.log_bucket.id
  target_prefix   = "log/"
}
  1. rm -rf ~/Library/Caches/trivy/policy
  2. go run ./cmd/trivy conf main.tf -d --policy-bundle-repository ghcr.io/nikpivkin/trivy-policies:test
2024-04-16T17:50:24+07:00       DEBUG   [misconf] 50:24.188902000 terraform.scanner.rego           Error occurred while parsing: Users/nikita/Library/Caches/trivy/policy/content/policies/cloud/policies/aws/s3/enable_logging.rego, Users/nikita/Library/Caches/trivy/policy/content/policies/cloud/policies/aws/s3/enable_logging.rego:33: rego_type_error: undefined ref: bucket.logging.is_enabled.value
        bucket.logging.is_enabled.value
                       ^
                       have: "is_enabled"
                       want (one of): ["enabled" "targetbucket"]. 
Try loading embedded policy.
2024-04-16T17:50:24+07:00       DEBUG   [misconf] 50:24.189063000 terraform.scanner.rego           Found embedded policy: checks/cloud/aws/s3/enable_logging.rego

Related issues

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

simar7
simar7 reviewed Apr 17, 2024
View reviewed changes
simar7
simar7 reviewed Apr 17, 2024
View reviewed changes
@nikpivkin nikpivkin marked this pull request as ready for review April 17, 2024 08:35
knqyf263
knqyf263 approved these changes Apr 17, 2024
View reviewed changes
Copy link
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked commands/artifact and scanner/local. They look good to me.

@nikpivkin nikpivkin mentioned this pull request Apr 18, 2024
Merged
@simar7
Copy link
Member

simar7 commented Apr 19, 2024 •
edited
Loading

@nikpivkin one case I found was if we ever have a bad check, the fallback to embedded scenario will continue to take place until a new bundle is downloaded (which includes a fixed check). This would occur on each Trivy run.

Perhaps this is acceptable. I thought about re-downloading the bundle in this case (in addition to falling back for that particular run) but I think it's complicated and error prone in other ways.

Thoughts?

@simar7 simar7 self-requested a review April 19, 2024 01:16
@nikpivkin
Copy link
Contributor Author

@simar7 You mean re-download the previous version's bundle? Then all checks will be rolled back, including those with no errors.

@simar7
Copy link
Member

simar7 commented Apr 19, 2024 •
edited
Loading

@simar7 You mean re-download the previous version's bundle? Then all checks will be rolled back, including those with no errors.

No I meant download the latest bundle available. But I think it adds complexity so let's not do it.

@simar7 simar7 added this pull request to the merge queue Apr 19, 2024
Merged via the queue into aquasecurity:main with commit 12ec0df Apr 19, 2024
13 checks passed
@nikpivkin nikpivkin deleted the fallback-check branch April 19, 2024 08:00
fl0pp5 pushed a commit to altlinux/trivy that referenced this pull request May 6, 2024
@nikpivkin @fl0pp5
feat(misconf): loading embedded checks as a fallback (aquasecurity#6502)
aa8e3e7
chen-keinan
chen-keinan reviewed May 6, 2024
View reviewed changes
pkg/iac/rego/load.go
var excludedFiles []string

for _, e := range compiler.Errors {
loc := e.Location.File
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikpivkin @simar7 I have bump trivy-operator with latest trivy 0.51.1 and I'm getting nil pointer exception on L152 while running my policies tests
loc := e.Location.File anyway to protect it?

here is link for test failure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Location can be nil. I'll add a check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chen-keinan Opened the PR with a fix. #6638

@nikpivkin nikpivkin mentioned this pull request May 6, 2024
Merged
6 tasks
@aqua-bot aqua-bot mentioned this pull request May 29, 2024
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@chen-keinan chen-keinan chen-keinan left review comments

@simar7 simar7 simar7 approved these changes

@knqyf263 knqyf263 knqyf263 approved these changes

@DmitriyLewen DmitriyLewen Awaiting requested review from DmitriyLewen DmitriyLewen is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat(misconf): Fallback to embedded check if needed
4 participants
@nikpivkin @simar7 @knqyf263 @chen-keinan